feat(compiler): allow SDL to contain built-in type redefinition #994
Conversation
Updates `SchemaBuilder` to allow SDL to contain built-in meta types (e.g. `__Type`) redefinitions. GraphQL spec doesn't really define the expected behavior for handling SDL with redeclared meta fields (see: [#1036](graphql/graphql-spec#1036), [#1049](graphql/graphql-spec#1049)). `graphql-js` [reference implementation](https://github.com/graphql/graphql-js/blob/v16.11.0/src/utilities/extendSchema.ts#L187) simply ignores those redefinitions even if the underlying type definitions don't match. This PR updates `ignore_builtin_redefinitions` option to also ignore meta types redefinitions (previously it was only allowing built-in scalar redefinitions #990).
* moves tests to their own file * adds all scalars and types to the test
| "#; | ||
|
|
||
| let errors = Schema::parse_and_validate(schema, "schema.graphql") | ||
| .expect_err("valid schema") |
There was a problem hiding this comment.
i changed the message here (before it said "invalid schema"). expect_err panics when an Err is expected but we got an Ok. So here, instead of getting an invalid schema, we actually get a valid one.
There was a problem hiding this comment.
ugh, no, ignore me, invalid schema was correct! i'll change this back in a follow up PR
There was a problem hiding this comment.
Fyi the meta type definition for __Directive was incorrect (on purpose) with nullable arg vs non nun list of non null args. This was to show that we just ignore those redefinitions even if they dont match.
There was a problem hiding this comment.
I left it as incorrect, and all the other types I added were also slightly incorrect. A bit harder with scalars, so scalars are all the same.
| } | ||
|
|
||
| #[test] | ||
| fn handles_built_in_type_redefinition() { |
There was a problem hiding this comment.
This types.rs file was for ValuesOfCorrectType Rule tests, so I moved these builtin_type_redefinition tests into their own file.
| #[test] | ||
| fn handles_built_in_scalar_redefinition() { | ||
| let schema = r#" | ||
| scalar String |
There was a problem hiding this comment.
So we are extra sure, I added all the scalar redefinitions and all the type redefinitions (below).
Updates
SchemaBuilderto allow SDL to contain built-in meta types (e.g.__Type) redefinitions. GraphQL spec doesn't really define the expected behavior for handling SDL with redeclared meta fields (see: #1036, #1049).graphql-jsreference implementation simply ignores those redefinitions even if the underlying type definitions don't match.This PR updates
ignore_builtin_redefinitionsoption to also ignore meta types redefinitions (previously it was only allowing built-in scalar redefinitions #990).